Conversation
…verRegistry interface
toinehartman
left a comment
toinehartman
left a comment
There was a problem hiding this comment.
Impressive amount of work again! Although I trust your local testing, I think it would be nice to see an all-green CI by using a test-only snapshot release of Rascal if possible.
| .filter(s => !this.protectedSchemes.includes(s)) | ||
| // we add support for schemes that look inside a jar | ||
| .concat(schemes | ||
| .filter(s => s !== "jar" && s !== "zip" && s !== "compressed") |
There was a problem hiding this comment.
compressed:? Never seen that before. How can that be used?
There was a problem hiding this comment.
It's for dynamically decompressing a file. It's used for large data sets for example: readFile(|compressed+file:///big-data-set.csv.gz|) or readCSV(|compressed+home:///big-data-set.csv.gz|);
| readDirectory(uri: vscode.Uri): [string, vscode.FileType][] | Thenable<[string, vscode.FileType][]> { | ||
| this.logger.trace("[RascalFileSystemInVSCode] readDirectory: ", uri); | ||
| return this.sendRequest<DirectoryListingResponse>(uri, "rascal/vfs/input/list") | ||
| .then(c => c.entries.map(ft => [ft.name, ft.types.reduce((a, i) => a | i)])); |
There was a problem hiding this comment.
types is an (admittedly mostly singleton) array of file types (e.g., file and symbolic link), which are encoded as a single bit. The reduce converts the array into a bitmap.
Co-authored-by: Toine Hartman <toinehartman@users.noreply.github.com>
| @Override | ||
| public CompletableFuture<SourceLocationResponse> resolveLocation(ISourceLocationRequest req) { | ||
| logger.trace("resolveLocation: {}", req.getLocation()); | ||
| return super.resolveLocation(new ISourceLocationRequest(Locations.toClientLocation(req.getLocation()))); |
There was a problem hiding this comment.
how about we introduce a small utilty function that can reconstruct a ISourceLocationRequest as right now the Locations.toClientLocation is repeated in quite some places.
| * Wrapper for RascalFileSystemServices handling LSP-specifics. | ||
| * In particular, locations from LSP are mapped to Rascal-friendly locations, and Rascal exceptions are mapped to exceptions LSP expects. | ||
| */ | ||
| public class RascalFileSystemInVSCode extends RascalFileSystemServices { |
There was a problem hiding this comment.
can we add a comment that explains why not all calls over overriden? it looks to me like there should be more?
| .setOutput(out) | ||
| .configureGson(GsonUtils.complexAsJsonObject()) | ||
| .setExecutorService(threadPool) | ||
| .setExceptionHandler(t -> RemoteIOError.translate((Exception) t).getResponseError()) |
There was a problem hiding this comment.
we should only call this if it's an IOException as it might already be a RespondseException or something else.
I think we already have something like this to deal with exceptions that should be send to the VS Code UI for error reporting.
|
|
||
| async watch(newWatch: WatchRequest): Promise<void> { | ||
| this.logger.trace("[VSCodeFileSystemInRascal] watch: ", newWatch.loc); | ||
| const watchKey = newWatch.loc + newWatch.recursive; |
There was a problem hiding this comment.
we should a charcter between the loc and the boolean, such that we never get colissions. for example:
const watchKey = `${newWatch.loc}##${newWatch.recursive}`;There was a problem hiding this comment.
How could there a collision here?
There was a problem hiding this comment.
Hmmmm, I was thinking about a file named true or something, but that's not right, it'll still het the boolean suffix.
| this.disposables.push(watcher); | ||
| return; | ||
| } | ||
| throw new rpc.ResponseError(RemoteIOError.watchAlreadyDefined, `Watch already defined: ${newWatch.loc}`); |
There was a problem hiding this comment.
this only works because we provide some guarantees of never being called twice. I think we should document some of that on the java classes.
…id code duplication
3 participants
No description provided.